Skip to content

Eliminate statSync calls and filter prefixes first in path completion - #1214

Open
nordicnode wants to merge 1 commit into
CodebuffAI:mainfrom
nordicnode:perf-path-completion-dirents
Open

Eliminate statSync calls and filter prefixes first in path completion#1214
nordicnode wants to merge 1 commit into
CodebuffAI:mainfrom
nordicnode:perf-path-completion-dirents

Conversation

@nordicnode

Copy link
Copy Markdown

Eliminate statSync calls and filter prefixes first in path completion

Summary

• In cli/src/utils/path-completion.ts, optimize getPathCompletion to eliminate $O(N)$ synchronous disk statSync calls during CLI tab completion.
• Previously, getPathCompletion called readdirSync(parentDir) and immediately ran statSync(fullPath).isDirectory() on every single item in the directory before checking if it matched the typed prefix. In directories with thousands of files (e.g. user home, desktop, monorepo roots), this executed thousands of synchronous filesystem syscalls on every Tab press, freezing the TUI.
• Switched to readdirSync(parentDir, { withFileTypes: true }) and placed the prefix match check (!name.toLowerCase().startsWith(partial)) first. For matching items, entry.isDirectory() checks inode type directly with zero extra syscalls, only falling back to statSync when a symbolic link needs resolution.
• Benchmark: In a directory of 2,500 files, reduced statSync syscalls from 2,505 per Tab completion down to 0, improving completion latency by 6.2x (102.56ms -> 16.55ms across 20 iterations).
• Adds regression unit test in cli/src/__tests__/path-completion.test.ts verifying that symlinked directories continue to resolve and complete correctly.

Test plan

[✓] bun test --config=/dev/null src/__tests__/path-completion.test.ts — 25 pass, 0 fail
[✓] bun test --config=/dev/null src/hooks/__tests__/use-path-tab-completion.test.ts — 33 pass, 0 fail
[✓] bun run --cwd cli typecheck — 0 errors
[✓] PR hygiene check passed

@codebuff-team

Copy link
Copy Markdown
Contributor

Good catch and clean fix. Moving the prefix filter before the directory check in getPathCompletion (cli/src/utils/path-completion.ts) is the right order of operations - no reason to touch the filesystem for entries that don't even match the typed prefix. Switching to readdirSync(parentDir, { withFileTypes: true }) avoids a statSync per entry for the common case, falling back to statSync only when entry.isSymbolicLink() is true, which correctly preserves symlink-to-directory completion behavior (and the broken-symlink case still safely resolves to isDirectory = false via the try/catch).

The added test in path-completion.test.ts covers the symlink resolution path, which is exactly the risky edge case this refactor touches - good instinct to add it rather than relying on manual verification.

Small things worth double-checking before this lands: confirm partial is already lowercased upstream (the diff assumes name.toLowerCase().startsWith(partial) matches prior semantics, which it appears to), and consider whether hidden-dotfile filtering interacts correctly with the new early-continue ordering (looks fine from the diff, just worth a second look during porting).

Overall this is a small, well-targeted, and tested change to a real performance bug (O(N) sync stat calls freezing the TUI on large directories). Worth porting.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree labels Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants